Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work with pools that don't support prepared statements #1147

Merged
merged 6 commits into from
Jul 13, 2024

Conversation

ramnivas
Copy link
Contributor

@ramnivas ramnivas commented Jun 4, 2024

Introduce a new query_with_param_types method that allows to specify Postgres type parameters. This obviated the need to use prepared statements just to obtain parameter types for a query. It then combines parse, bind, and execute in a single packet.

Related: #1017, #1067

Introduce a new `query_with_param_types` method that allows to specify Postgres type parameters. This obviated the need to use prepared statementsjust to obtain parameter types for a query. It then combines parse, bind, and execute in a single packet.

Related: sfackler#1017, sfackler#1067
) -> impl ExactSizeIterator<Item = (&'a dyn ToSql, Type)> + 'a {
s.iter()
.map(|(param, param_type)| (*param as _, param_type.clone()))
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to factor this into a separate function here since it's only being called one place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Earlier thought was to allow access to the raw RowStream.

responses,
rows_affected: None,
_p: PhantomPinned,
})
}

enum QueryProcessingState {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like this is a lot of boilerplate over a few match responses.next().await? { ... } in a sequence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@sfackler
Copy link
Owner

sfackler commented Jul 7, 2024

Thinking about it, it seems like we could make query_typed do this when invoked with a non-prepared statement instead of adding a new method.

@ramnivas
Copy link
Contributor Author

ramnivas commented Jul 7, 2024

Thinking about it, it seems like we could make query_typed do this when invoked with a non-prepared statement instead of adding a new method.

Did you mean prepare_typed (there is no query_typed). One nice thing about the new query_with_param_types is that it takes param values and param types through tuples, so the client cannot specify fewer (or more) types than parameter length.

@sfackler
Copy link
Owner

sfackler commented Jul 9, 2024

Did you mean prepare_typed (there is no query_typed).

Ah yeah my idea didn't really make much sense :D

@sfackler
Copy link
Owner

sfackler commented Jul 9, 2024

Let's rename the method to query_typed, which aligns with prepare_typed and is a bit shorter.

@ramnivas
Copy link
Contributor Author

ramnivas commented Jul 9, 2024

Let's rename the method to query_typed, which aligns with prepare_typed and is a bit shorter.

Done!

@sfackler
Copy link
Owner

LGTM but there's one small clippy error to fix.

@ramnivas
Copy link
Contributor Author

LGTM but there's one small clippy error to fix.

Fixed.

@sfackler sfackler merged commit 257bcfd into sfackler:master Jul 13, 2024
4 checks passed
@sfackler
Copy link
Owner

Thanks!

@ramnivas
Copy link
Contributor Author

Thanks!

Thank you!

ramnivas added a commit to exograph/exograph that referenced this pull request Sep 18, 2024
With sfackler/rust-postgres#1165 merged and a new release with that change available in 0.7.12, we no longer need to use our fork.

Compared to sfackler/rust-postgres#1147, there was a name change for `query_typed`, so this commit takes care of that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants